Use "capacity" as parameter name in with_capacity() methods #60316

mgeier · 2019-04-26T16:50:23Z

The only place where I didn't change the parameter name is RawVec. The problem is that it has a .cap() method instead of the usual .capacity():

rust/src/liballoc/raw_vec.rs

Lines 200 to 210 in 597f432

    
           /// Gets the capacity of the allocation. 
        
           /// 
        
           /// This will always be `usize::MAX` if `T` is zero-sized. 
        
           #[inline(always)] 
        
           pub fn cap(&self) -> usize { 
        
               if mem::size_of::<T>() == 0 { 
        
                   !0 
        
               } else { 
        
                   self.cap 
        
               } 
        
           }

Changing this would be a breaking change, and I guess that's not worth it.

But since I didn't change .cap() there, I didn't change the cap parameter name to capacity, either.

Closes rust-lang#60271.

rust-highfive · 2019-04-26T16:50:33Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

mgeier · 2019-04-26T17:39:56Z

I've found two more evil .cap() methods:

rust/src/libstd/sync/mpsc/sync.rs

Lines 461 to 479 in 597f432

    
           impl<T> Buffer<T> { 
        
               fn enqueue(&mut self, t: T) { 
        
                   let pos = (self.start + self.size) % self.buf.len(); 
        
                   self.size += 1; 
        
                   let prev = mem::replace(&mut self.buf[pos], Some(t)); 
        
                   assert!(prev.is_none()); 
        
               } 
        
               fn dequeue(&mut self) -> T { 
        
                   let start = self.start; 
        
                   self.size -= 1; 
        
                   self.start = (self.start + 1) % self.buf.len(); 
        
                   let result = &mut self.buf[start]; 
        
                   result.take().unwrap() 
        
               } 
        
               fn size(&self) -> usize { self.size } 
        
               fn cap(&self) -> usize { self.buf.len() } 
        
           }

rust/src/liballoc/collections/vec_deque.rs

Lines 87 to 103 in 597f432

    
           impl<T> VecDeque<T> { 
        
               /// Marginally more convenient 
        
               #[inline] 
        
               fn ptr(&self) -> *mut T { 
        
                   self.buf.ptr() 
        
               } 
        
               /// Marginally more convenient 
        
               #[inline] 
        
               fn cap(&self) -> usize { 
        
                   if mem::size_of::<T>() == 0 { 
        
                       // For zero sized types, we are always at maximum capacity 
        
                       MAXIMUM_ZST_CAPACITY 
        
                   } else { 
        
                       self.buf.cap() 
        
                   } 
        
               }

joshtriplett · 2019-04-26T17:52:31Z

@bors r+

bors · 2019-04-26T17:52:32Z

📌 Commit be12ab0 has been approved by joshtriplett

estebank · 2019-04-26T18:16:30Z

@bors rollup

…=joshtriplett Use "capacity" as parameter name in with_capacity() methods See rust-lang#60271. The only place where I didn't change the parameter name is `RawVec`. The problem is that it has a `.cap()` method instead of the usual `.capacity()`: https://github.com/rust-lang/rust/blob/597f432489f12a3f33419daa039ccef11a12c4fd/src/liballoc/raw_vec.rs#L200-L210 Changing this would be a breaking change, and I guess that's not worth it. But since I didn't change `.cap()` there, I didn't change the `cap` parameter name to `capacity`, either.

@ghost

Rollup of 5 pull requests Successful merges: - #60292 (Replace the `&'tcx List<Ty<'tcx>>` in `TyKind::Tuple` with `SubstsRef<'tcx>`) - #60307 (Make "Implementations on Foreign Types" items in sidebar link to specific impls) - #60309 (Add 1.34.1 release notes) - #60315 (bootstrap: use correct version numbers for llvm-tools and lldb) - #60316 (Use "capacity" as parameter name in with_capacity() methods) Failed merges: r? @ghost

bors · 2019-04-27T16:57:53Z

☔ The latest upstream changes (presumably #60329) made this pull request unmergeable. Please resolve the merge conflicts.

Rename .cap() methods to .capacity() As mentioned in rust-lang#60316, there are a few `.cap()` methods, which seem out-of-place because such methods are called `.capacity()` in the rest of the code. This PR renames them to `.capacity()` but leaves `RawVec::cap()` in there for backwards compatibility. I didn't try to mark the old version as "deprecated", because I guess this would cause too much noise.

Rename .cap() methods to .capacity() As mentioned in #60316, there are a few `.cap()` methods, which seem out-of-place because such methods are called `.capacity()` in the rest of the code. This PR renames them to `.capacity()` but leaves `RawVec::cap()` in there for backwards compatibility. I didn't try to mark the old version as "deprecated", because I guess this would cause too much noise.

Use "capacity" as parameter name in with_capacity() methods

be12ab0

Closes rust-lang#60271.

rust-highfive assigned joshtriplett Apr 26, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 26, 2019

mgeier mentioned this pull request Apr 26, 2019

Inconsistent parameter name "capacity" #60271

Closed

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 26, 2019

Centril mentioned this pull request Apr 27, 2019

Rollup of 5 pull requests #60324

Closed

Centril mentioned this pull request Apr 27, 2019

Rollup of 6 pull requests #60328

Closed

Centril mentioned this pull request Apr 27, 2019

Rollup of 5 pull requests #60329

Merged

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 27, 2019

bors merged commit be12ab0 into rust-lang:master Apr 27, 2019

mgeier deleted the rename-capacity-parameter branch April 27, 2019 18:22

mgeier mentioned this pull request Apr 27, 2019

Rename .cap() methods to .capacity() #60340

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use "capacity" as parameter name in with_capacity() methods #60316

Use "capacity" as parameter name in with_capacity() methods #60316

mgeier commented Apr 26, 2019

rust-highfive commented Apr 26, 2019

mgeier commented Apr 26, 2019

joshtriplett commented Apr 26, 2019

bors commented Apr 26, 2019

estebank commented Apr 26, 2019

bors commented Apr 27, 2019

	/// Gets the capacity of the allocation.
	///
	/// This will always be `usize::MAX` if `T` is zero-sized.
	#[inline(always)]
	pub fn cap(&self) -> usize {
	if mem::size_of::<T>() == 0 {
	!0
	} else {
	self.cap
	}
	}

Use "capacity" as parameter name in with_capacity() methods #60316

Use "capacity" as parameter name in with_capacity() methods #60316

Conversation

mgeier commented Apr 26, 2019

rust-highfive commented Apr 26, 2019

mgeier commented Apr 26, 2019

joshtriplett commented Apr 26, 2019

bors commented Apr 26, 2019

estebank commented Apr 26, 2019

bors commented Apr 27, 2019